core: RetryPolicy total sleep budget (maxTotalDelay) and builder - #78
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RetryPolicybounded each sleep (maxDelay, also theRetry-Afterceiling) and the attempt count, but nothing bounded their sum: raisemaxDelayto bridge a per-minute window andmaxAttemptsto five, and one call can hold the caller's thread for minutes of sleeping. ADR-027 (plan decision D3) addsmaxTotalDelay— default 1 min, the worst case the other defaults already allowed, so behaviour at the defaults is unchanged — checked before every sleep: a sleep that would exceed the budget is never started, retrying ends and the exception surfaces with its hint preserved, mirroring the ADR-025 ceiling. Exactly-reached budgets are honoured.DurationaftermaxDelay(seven components). Migration:RetryPolicy.builder()(new — starts fromdefaults(), one setter per knob, the path that survives future knobs), or insertDuration.ofMinutes(1)as the fourth argument. No compatibility constructor, by decision: it would keep the positional trap public beside the builder meant to end it. Zero external consumers today.fanar.retry.max-total-delay(default1m);fanarRetryPolicyis built through the builder, so the four knobs validate together and amax-delayraised above the budget fails the context at startup with the policy's own message. ADR-014 / ADR-020 amended; ARCHITECTURE, API_SKETCH §10–11, sampleapplication.yml, PROJECT_STATE and CHANGELOG updated.Type of change
Test plan
mvn verifypasses locallyFANAR_API_KEYset)--self-testpasses (if the SDK's reflective surface changed)Reviewer checklist
*IntegrationTestproves the behaviour this PR claims? —FanarClientRetryIntegrationTest.retryAfterHintsBeyondTheTotalBudgetEndRetrying(public builder → chain → scripted server: two 429s withRetry-After: 1under a 1 s ceiling and 1 s budget — first slept, second refused, hint preserved,fanar.retry_count=1, no further request); loop semantics inRetryInterceptorTest(totalDelayBudget*,retryAfterHintBeyondTheRemainingBudget*); starter inFanarAutoConfigurationTest(retryKnobsAreValidatedTogetherAtStartup, knob default + override)@Deprecated(since, forRemoval = true)and a migration path in the PR descriptionqa.fanar.corepublic APImemory/so future contributors find it